[Feature]: Save additional metrics to SR report for all annotations #460

abhijith-trenser · 2025-10-28T10:06:45Z

Context

This PR implements the feature described in [Feature Request] Save additional metrics to SR report for all annotations #459 .
It is part of the corresponding PR in cs3d: [Feature]: Save additional metrics to SR report for all annotations cornerstonejs/cornerstone3D#2430
This PR has been incorporated by FlyWheel.io

Changes & Results

Added Radius, Mean, Maximum, and Minimum metrics to TID300 for Circle, Ellipse, and Polyline tools.
Introduced a helper function buildContentSequence to eliminate repetitive graphic data usage across tools.

What are the effects of this change?

The following tools in Cornerstone3D, Length, Bidirectional, Ellipse, Circle, Rectangle, and Freehand now include all annotation metrics without requiring rendering.

This feature applies only to newly created SRs. Existing SR files will not reflect these changes unless re-saved.

Testing

Launch a Study using /local
Draw annotations in different slices using different tools.
Open right side panel
Click 'Create SR' and save the SR with a file name.
Launch the same study along with the downloaded SR
Load the SR into viewport and confirm to track the study
Observe the annotations and all annotaions display all metrics without rendering it.

Checklist

PR

My Pull Request title is descriptive, accurate and follows the
semantic-release format and guidelines.

Code

My code has been well-documented (function documentation, inline comments,
etc.)

Public Documentation Updates

The documentation page has been updated as necessary for any public API
additions or removals.

Tested Environment

OS: Windows 11
Node version: 22
Browser: Chrome 141.0.7390.123

netlify · 2025-10-28T10:06:55Z

✅ Deploy Preview for dcmjs2 ready!

Name	Link
🔨 Latest commit	`4e0f2e2`
🔍 Latest deploy log	https://app.netlify.com/projects/dcmjs2/deploys/6929bb9206dbc100087bb6f5
😎 Deploy Preview	https://deploy-preview-460--dcmjs2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

sen-trenser · 2025-11-10T12:21:38Z

@sedghi Could you please review this or suggest anyone that can help with approving the PR in dcmjs?
This is related to cornerstonejs/cornerstone3D#2392

wayfarer3130 · 2025-11-17T13:57:40Z

@sen-trenser - could you:

Attach an example dcm file for a planar freeform with 2 different types of measurement values?
Run dciodvfy on the example dcm file
Annotate a dcmdump verison of the example with your changes?
It would just help a lot to have it well documented as to what the changes look like in terms of generated output.

nithin-trenser · 2025-11-18T14:43:14Z

@wayfarer3130 We found some errors when run dciodvfy on the example dcm file. Currently resolving these errors.

nithin-trenser · 2025-11-19T16:17:03Z

@wayfarer3130
Please find the attachment below.

Attach an example dcm file for a planar freeform with 2 different types of measurement values?
- Example dcm files are attached Example_files.zip
Annotate a dcmdump verison of the example with your changes?
- Annotated dcmdump version of example file -> sr-all-annotations.txt
  cc: @sen-trenser

sen-trenser · 2025-11-24T06:53:04Z

@wayfarer3130 We had some modifications to fix errors reported by the dciodvfy tool. Could you please check the attached data @nithin-trenser provided above?

Please check the area marked with text #################### in the sr-all-annotations.txt to see the new content changes.

wayfarer3130 · 2025-11-24T15:30:09Z

There is the concept of INFERRED FROM allowing references to a common layout to be used - that basically looks like below:
Note the ReferencedContentItemIdentifier
It is legal to put those under ONLY the second and subsequent measurement. That isn't the "preferred" DICOM way, but it is legal DICOM as far as I know, and I believe is compliant with our parsing. The problem with just repeating data is that there isn't a way to tell it is actually the same/consistent data, and there are circumstances when it actually uses different data.
Would it be possible for you to make that tweak as well to reference the first set of data?

(0040,a730) SQ (Sequence with undefined length #=3) # u/l, 1 ContentSequence
(fffe,e000) na (Item with undefined length #=7) # u/l, 1 Item 1: SCOORD ROI
(0040,a010) CS [CONTAINS] # 10, 1 RelationshipType
(0040,a040) CS [SCOORD] # 6, 1 ValueType
(0040,a043) SQ (Sequence with undefined length #=1) # u/l, 1 ConceptNameCodeSequence
(fffe,e000) na (Item with undefined length #=3) # u/l, 1 Item
(0008,0100) SH [121071] # 6, 1 CodeValue (example: “Region of Interest”)
(0008,0102) SH [DCM] # 4, 1 CodingSchemeDesignator
(0008,0104) LO [ROI] # 4, 1 CodeMeaning
(fffe,e00d) na (ItemDelimitationItem) # 0, 0
(fffe,e0dd) na (SequenceDelimitationItem) # 0, 0
(0040,a730) SQ (Sequence with undefined length #=1) # u/l, 1 ContentSequence (child: IMAGE)
(fffe,e000) na (Item with undefined length #=4) # u/l, 1 Item 1.1: IMAGE
(0040,a010) CS [SELECTED FROM] # 14, 1 RelationshipType
(0040,a040) CS [IMAGE] # 6, 1 ValueType
(0008,1199) SQ (Sequence with undefined length #=1) # u/l, 1 ReferencedSOPSequence
(fffe,e000) na (Item with undefined length #=2) # u/l, 1 Item
(0008,1150) UI =CTImageStorage # 26, 1 ReferencedSOPClassUID
(0008,1155) UI [1.2.3.4.5.6.7.8.9] # xx, 1 ReferencedSOPInstanceUID (PUT YOUR UID HERE)
(fffe,e00d) na (ItemDelimitationItem) # 0, 0
(fffe,e0dd) na (SequenceDelimitationItem) # 0, 0
(fffe,e00d) na (ItemDelimitationItem) # 0, 0
(fffe,e0dd) na (SequenceDelimitationItem) # 0, 0
(0070,0022) FL 202.8033\60.46706\202.8033\114.96518\193.98744\87.716118\211.61917 # 32, 8 GraphicData
(0070,0023) CS [ELLIPSE] # 8, 1 GraphicType
(fffe,e00d) na (ItemDelimitationItem) # 0, 0 ItemDelimitationItem

(fffe,e000) na (Item with undefined length #=6) # u/l, 1 Item 2: NUM Measurement #1
(0040,a010) CS [CONTAINS] # 10, 1 RelationshipType
(0040,a040) CS [NUM] # 4, 1 ValueType
(0040,a043) SQ (Sequence with undefined length #=1) # u/l, 1 ConceptNameCodeSequence
(fffe,e000) na (Item with undefined length #=3) # u/l, 1 Item
(0008,0100) SH [121206] # 6, 1 CodeValue (example: “Length”)
(0008,0102) SH [DCM] # 4, 1 CodingSchemeDesignator
(0008,0104) LO [Length] # 6, 1 CodeMeaning
(fffe,e00d) na (ItemDelimitationItem) # 0, 0
(fffe,e0dd) na (SequenceDelimitationItem) # 0, 0
(0040,a300) SQ (Sequence with undefined length #=1) # u/l, 1 MeasuredValueSequence
(fffe,e000) na (Item with undefined length #=4) # u/l, 1 Item
(0040,a30a) DS [12.3] # 4, 1 NumericValue (PUT YOUR VALUE)
(0040,08ea) SQ (Sequence with undefined length #=1) # u/l, 1 MeasurementUnitsCodeSequence
(fffe,e000) na (Item with undefined length #=3) # u/l, 1 Item
(0008,0100) SH [mm] # 2, 1 CodeValue
(0008,0102) SH [UCUM] # 4, 1 CodingSchemeDesignator
(0008,0104) LO [millimeter] # 10, 1 CodeMeaning
(fffe,e00d) na (ItemDelimitationItem) # 0, 0
(fffe,e0dd) na (SequenceDelimitationItem) # 0, 0
(fffe,e00d) na (ItemDelimitationItem) # 0, 0
(fffe,e0dd) na (SequenceDelimitationItem) # 0, 0
(0040,a730) SQ (Sequence with undefined length #=1) # u/l, 1 ContentSequence (evidence: by-reference)
(fffe,e000) na (Item with undefined length #=2) # u/l, 1 Item 2.1: by-reference to SCOORD
(0040,a010) CS [INFERRED FROM] # 14, 1 RelationshipType
(0040,db73) UL 1 # 4, 1 ReferencedContentItemIdentifier (REFERS TO ITEM 1)
(fffe,e00d) na (ItemDelimitationItem) # 0, 0
(fffe,e0dd) na (SequenceDelimitationItem) # 0, 0
(fffe,e00d) na (ItemDelimitationItem) # 0, 0 ItemDelimitationItem

(fffe,e000) na (Item with undefined length #=6) # u/l, 1 Item 3: NUM Measurement #2
(0040,a010) CS [CONTAINS] # 10, 1 RelationshipType
(0040,a040) CS [NUM] # 4, 1 ValueType
(0040,a043) SQ (Sequence with undefined length #=1) # u/l, 1 ConceptNameCodeSequence
(fffe,e000) na (Item with undefined length #=3) # u/l, 1 Item
(0008,0100) SH [121207] # 6, 1 CodeValue (example: “Area”)
(0008,0102) SH [DCM] # 4, 1 CodingSchemeDesignator
(0008,0104) LO [Area] # 4, 1 CodeMeaning
(fffe,e00d) na (ItemDelimitationItem) # 0, 0
(fffe,e0dd) na (SequenceDelimitationItem) # 0, 0
(0040,a300) SQ (Sequence with undefined length #=1) # u/l, 1 MeasuredValueSequence
(fffe,e000) na (Item with undefined length #=4) # u/l, 1 Item
(0040,a30a) DS [45.6] # 4, 1 NumericValue (PUT YOUR VALUE)
(0040,08ea) SQ (Sequence with undefined length #=1) # u/l, 1 MeasurementUnitsCodeSequence
(fffe,e000) na (Item with undefined length #=3) # u/l, 1 Item
(0008,0100) SH [mm2] # 3, 1 CodeValue
(0008,0102) SH [UCUM] # 4, 1 CodingSchemeDesignator
(0008,0104) LO [square millimeter] # 17, 1 CodeMeaning
(fffe,e00d) na (ItemDelimitationItem) # 0, 0
(fffe,e0dd) na (SequenceDelimitationItem) # 0, 0
(fffe,e00d) na (ItemDelimitationItem) # 0, 0
(fffe,e0dd) na (SequenceDelimitationItem) # 0, 0
(0040,a730) SQ (Sequence with undefined length #=1) # u/l, 1 ContentSequence (evidence: by-reference)
(fffe,e000) na (Item with undefined length #=2) # u/l, 1 Item 3.1: by-reference to SCOORD
(0040,a010) CS [INFERRED FROM] # 14, 1 RelationshipType
(0040,db73) UL 1 # 4, 1 ReferencedContentItemIdentifier (REFERS TO ITEM 1)
(fffe,e00d) na (ItemDelimitationItem) # 0, 0
(fffe,e0dd) na (SequenceDelimitationItem) # 0, 0
(fffe,e00d) na (ItemDelimitationItem) # 0, 0 ItemDelimitationItem

wayfarer3130 · 2025-11-24T16:12:32Z

@pieper - for the saving of SR annotations, I'm proposing that second and subsequent measurements use references to the annotation definition in the first measurement value - that is, a structure like:

measurement value 1:
max value MAX
annotation graphic data ....
measurement value 2:
min value: MIN
annotation reference: 1/1 (refers to first sub-sequence of the shared measurement value, starting from the shared parent.)

The TID 1500 canonical way I believe is:
annotation graphic data: ...
measurement value 1:
max value MAX
annotation reference: 1
measurement value 2:
min value MIN
annotation reference: 2

As I read part 16, I THINK either way is acceptable, but I'm not 100% sure. The point in either one is to avoid duplicating the image/graphic data references so that there is no chance that they are different. That is only done when they actually ARE the same - things like bidirectional are not the same annotations first/second.

Might ask David what he thinks about this? I'd very much prefer not to go down something not standards compliant.

nithin-trenser · 2025-12-01T11:33:54Z

@wayfarer3130
Please find attached the dcmdump for the new SR report generated with the ReferencedContentItemIdentifier tag. Could you take a look and provide your feedback?
all-annotations.txt
cc: @sen-trenser

wayfarer3130 · 2025-12-01T12:13:00Z

src/utilities/TID300/Circle.js

+        ];
+        if (radius) {
+            measurements.push({
+                RelationshipType: "CONTAINS",


I'm ok with this as it is, but it would be nice to have simple helper classes that just constructed this object via arguments, eg:
measurements.push(new RadiusReference(radius, annotationIndex))
Or maybe RadiusMeasuredValue.

wayfarer3130 · 2025-12-01T12:14:28Z

src/utilities/TID300/Ellipse.js

+        ];
+
+        if (max) {
+            measurements.push({


Again, all of these would become simpler if you just reference a structure class like MaximumValueReference(max, modalityUnit, annotationIndex)

wayfarer3130 · 2025-12-01T12:15:05Z

src/utilities/TID300/Point.js

            use3DSpatialCoordinates
        });

+        const graphicContentSequence = buildContentSequence({


I definitely find the new version easier to read

wayfarer3130 · 2025-12-01T12:16:45Z

src/utilities/TID300/Polyline.js

-        ]);
+        ];
+
+        if (max) {


You might even extract the full set of reference values here as a standard extractor for "area" measurements rather than including it in each measurement type. Again, not required, just suggesting ways to clean things up a bit more.

wayfarer3130 · 2025-12-01T12:23:44Z

src/utilities/TID300/buildContentSequence.js

+ * Builds a DICOM SR ContentSequence block for geometric measurements
+ * that share the same structure across tools (Circle, Ellipse, Polyline, etc.)
+ */
+export default function buildContentSequence({


@pieper - is there a better name for this that says exactly what this does? content sequence is a bit too generic.

Also, the general dcmjs pattern for this is as a constructor.

How about:
class Tid320ContentItem

constructor({
graphicType,
graphicData,
use3DSpatialCoordinates = false,
referencedSOPSequence,
referencedFrameOfReferenceUID}

and then assign to this.
I believe that meets the pattern.

wayfarer3130 · 2025-12-01T12:27:51Z

src/utilities/TID300/unit2CodingValue.js

    if (!codingUnit) {
-        log.error("Unspecified units", units);
-        return MM_UNIT;
+        return generateUnitMap(units);


Much as I like this change, it isnt actually valid and can result in ill defined units.
You can expand the list of UCUM units that are available, but not create a new one arbitrarily. If you want to do it this way, then use coding scheme designator "99dcmjsUnit and omit the scheme version, or set it to the version this got added in.

wayfarer3130

There are two nice to haves to extract the functionality for creating TID320 content items and sets of content items, and one must change to avoid creating invalid UCUM units.

saving additional metrics to SR

165c52e

abhijith-trenser marked this pull request as draft October 28, 2025 10:11

abhijith-trenser mentioned this pull request Oct 28, 2025

[Feature]: Save additional metrics to SR report for all annotations cornerstonejs/cornerstone3D#2430

Open

6 tasks

added buildContentSequence support for all tools

cc8b84b

abhijith-trenser marked this pull request as ready for review October 28, 2025 12:15

fixed errors on generated dicom files getting verified on dciodvfy

745816a

Added additional units support

db08d7d

nithin-trenser added 2 commits November 26, 2025 20:03

Used ReferencedContentItemIdentifier

f1c2008

corrected ReferencedContentItemIdentifier

4e0f2e2

wayfarer3130 reviewed Dec 1, 2025

View reviewed changes

wayfarer3130 requested changes Dec 1, 2025

View reviewed changes

[Feature]: Save additional metrics to SR report for all annotations #460

Are you sure you want to change the base?

[Feature]: Save additional metrics to SR report for all annotations #460

Conversation

abhijith-trenser commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Changes & Results

What are the effects of this change?

Testing

Checklist

PR

Code

Public Documentation Updates

Tested Environment

Uh oh!

netlify bot commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for dcmjs2 ready!

Uh oh!

sen-trenser commented Nov 10, 2025

Uh oh!

wayfarer3130 commented Nov 17, 2025

Uh oh!

nithin-trenser commented Nov 18, 2025

Uh oh!

nithin-trenser commented Nov 19, 2025

Uh oh!

sen-trenser commented Nov 24, 2025

Uh oh!

wayfarer3130 commented Nov 24, 2025

Uh oh!

wayfarer3130 commented Nov 24, 2025

Uh oh!

nithin-trenser commented Dec 1, 2025

Uh oh!

wayfarer3130 Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

wayfarer3130 Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

wayfarer3130 Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

wayfarer3130 Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

wayfarer3130 Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

wayfarer3130 Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

wayfarer3130 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

abhijith-trenser commented Oct 28, 2025 •

edited

Loading

netlify bot commented Oct 28, 2025 •

edited

Loading